Skip to content

chore: cli cleanup#323

Merged
calvinbrewer merged 1 commit intomainfrom
cli-fix-1
Mar 17, 2026
Merged

chore: cli cleanup#323
calvinbrewer merged 1 commit intomainfrom
cli-fix-1

Conversation

@calvinbrewer
Copy link
Contributor

@calvinbrewer calvinbrewer commented Mar 17, 2026

Summary by CodeRabbit

  • New Features

    • Improved CLI initialization flow with better handling of environment configuration.
    • Enhanced installation process for dependencies with clearer user guidance.
  • Chores

    • Minor version bumps for stack-forge and stack packages.

@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2026

🦋 Changeset detected

Latest commit: 5245cd7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@cipherstash/stack-forge Minor
@cipherstash/stack Minor
@cipherstash/basic-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

These changes refactor the CLI initialization flow by making DATABASE_URL a required environment variable check (removing interactive collection), introducing a conditional installation helper function, and expanding the installation step to handle both stack and stack-forge packages with production and development install commands.

Changes

Cohort / File(s) Summary
Changeset
.changeset/nice-pugs-retire.md
Adds changelog entry documenting minor version bumps for CLI setup and initialization command improvements.
Stack-Forge CLI Initialization
packages/stack-forge/src/commands/init.ts
Removes interactive DATABASE_URL prompt, adds pre-check for DATABASE_URL environment variable; config generation now happens unconditionally but installation prompt only proceeds if DATABASE_URL is set.
Stack Package Initialization
packages/stack/src/bin/commands/init/types.ts, packages/stack/src/bin/commands/init/utils.ts, packages/stack/src/bin/commands/init/steps/install-forge.ts
Adds stackInstalled field to InitState, introduces prodInstallCommand utility for package manager-specific install commands, implements installIfNeeded helper for conditional package installation, and expands installation step to handle both stack and stack-forge packages with appropriate prod/dev commands; step name updated from "Install stack-forge" to "Install stack dependencies".

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI as CLI Init
    participant Env as Environment
    participant FS as File System
    participant PM as Package Manager

    User->>CLI: Run init command
    CLI->>Env: Check DATABASE_URL
    alt DATABASE_URL not set
        CLI->>User: Display guidance note
        CLI-->>User: Exit setup
    else DATABASE_URL set
        CLI->>FS: Generate stash.config.ts
        FS-->>CLI: Config created
        CLI->>PM: Check if `@cipherstash/stack` installed
        alt Stack not installed
            CLI->>User: Prompt to install stack
            alt User confirms
                CLI->>PM: Run prod install command
                PM-->>CLI: Install complete
            else User cancels
                CLI-->>User: Continue without installing
            end
        end
        CLI->>PM: Check if `@cipherstash/stack-forge` installed
        alt Forge not installed
            CLI->>User: Prompt to install stack-forge
            alt User confirms
                CLI->>PM: Run dev install command
                PM-->>CLI: Install complete
            else User cancels
                CLI-->>User: Continue without installing
            end
        end
        CLI-->>User: Setup complete
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 The setup rabbit hops with care,
Checking if the database layer's there,
Stack and Forge in harmony installed,
With conditional prompts, we've called,
A smoother init, refined and clean!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'chore: cli cleanup' is vague and does not clearly convey the main changes, which involve improving CLI setup/initialization commands and refactoring the installation flow. Consider using a more descriptive title that reflects the primary change, such as 'chore: refactor CLI setup flow to separate database configuration and installation steps' or 'chore: improve CLI init command with conditional package installation'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cli-fix-1
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.

Change the reviews.profile setting to assertive to make CodeRabbit's nitpick more issues in your PRs.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/stack/src/bin/commands/init/steps/install-forge.ts (1)

54-58: Consider sanitizing error messages before logging.

The error message at line 57 could potentially include sensitive information if the install command or environment contains credentials. While the command itself is just package installation, error messages from package managers can sometimes include environment details.

🛡️ Optional: Sanitize error output
   } catch (err) {
-    const message = err instanceof Error ? err.message : String(err)
     s.stop(`${packageName} installation failed`)
-    p.log.error(message)
+    p.log.error(`Failed to install ${packageName}. Check your network and permissions.`)
     p.note(`You can install it manually:\n  ${cmd}`, 'Manual Installation')
     return false
   }

As per coding guidelines: "Do NOT log plaintext; the library never logs plaintext by design and logs should never leak sensitive data."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/stack/src/bin/commands/init/steps/install-forge.ts` around lines 54
- 58, The catch block logs raw error text which may leak secrets; change the
error logging in the catch to pass the error through a sanitizer helper (e.g.,
sanitizeErrorMessage) before calling p.log.error and only emit a short,
non-sensitive summary via s.stop/p.log.error while keeping p.note as-is;
specifically, update the catch handling around packageName, cmd, s.stop,
p.log.error to build a redactedMessage = sanitizeErrorMessage(err instanceof
Error ? err.message : String(err)) that strips/masks URLs with credentials,
tokens (token=, auth=, password=), long hex strings, and environment variable
values, then log a generic failure plus the redactedMessage rather than the raw
message.
packages/stack-forge/src/commands/init.ts (1)

113-126: Step number comment is inconsistent.

The comment at line 147 says "// 6. Determine install flags..." but there's no step 5. The step numbering jumps from 4 to 6.

🔧 Fix step numbering
-  // 6. Determine install flags from database provider
+  // 5. Determine install flags from database provider
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/stack-forge/src/commands/init.ts` around lines 113 - 126, The step
numbering in the inline comments is inconsistent: after the block that creates
CONFIG_FILENAME the next step comment jumps from "// 4. Install EQL
extensions..." to a later comment labeled "// 6. Determine install flags...";
update the comment text near the install-flag logic (the comment that currently
reads "// 6. Determine install flags...") to "// 5. Determine install flags..."
and scan the subsequent comments in the same function (e.g., surrounding
generateConfig, CONFIG_FILENAME, p.note, p.outro usages) to ensure all step
numbers are sequentially corrected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/stack-forge/src/commands/init.ts`:
- Around line 113-126: The step numbering in the inline comments is
inconsistent: after the block that creates CONFIG_FILENAME the next step comment
jumps from "// 4. Install EQL extensions..." to a later comment labeled "// 6.
Determine install flags..."; update the comment text near the install-flag logic
(the comment that currently reads "// 6. Determine install flags...") to "// 5.
Determine install flags..." and scan the subsequent comments in the same
function (e.g., surrounding generateConfig, CONFIG_FILENAME, p.note, p.outro
usages) to ensure all step numbers are sequentially corrected.

In `@packages/stack/src/bin/commands/init/steps/install-forge.ts`:
- Around line 54-58: The catch block logs raw error text which may leak secrets;
change the error logging in the catch to pass the error through a sanitizer
helper (e.g., sanitizeErrorMessage) before calling p.log.error and only emit a
short, non-sensitive summary via s.stop/p.log.error while keeping p.note as-is;
specifically, update the catch handling around packageName, cmd, s.stop,
p.log.error to build a redactedMessage = sanitizeErrorMessage(err instanceof
Error ? err.message : String(err)) that strips/masks URLs with credentials,
tokens (token=, auth=, password=), long hex strings, and environment variable
values, then log a generic failure plus the redactedMessage rather than the raw
message.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e76ea215-4143-4ca3-b695-a9823f9b95c9

📥 Commits

Reviewing files that changed from the base of the PR and between 8020f28 and 5245cd7.

📒 Files selected for processing (5)
  • .changeset/nice-pugs-retire.md
  • packages/stack-forge/src/commands/init.ts
  • packages/stack/src/bin/commands/init/steps/install-forge.ts
  • packages/stack/src/bin/commands/init/types.ts
  • packages/stack/src/bin/commands/init/utils.ts

@calvinbrewer calvinbrewer merged commit 68626b3 into main Mar 17, 2026
6 checks passed
@calvinbrewer calvinbrewer deleted the cli-fix-1 branch March 17, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant